Skip to content

NEW @W-21365643@ - Optimize SFGE Path Expansion Rule Performance#430

Merged
nikhil-mittal-165 merged 4 commits intofeature/static-rule-optimizationfrom
feature/sfge-thread-tuning
Mar 12, 2026
Merged

NEW @W-21365643@ - Optimize SFGE Path Expansion Rule Performance#430
nikhil-mittal-165 merged 4 commits intofeature/static-rule-optimizationfrom
feature/sfge-thread-tuning

Conversation

@nikhil-mittal-165
Copy link
Contributor

@nikhil-mittal-165 nikhil-mittal-165 commented Mar 6, 2026 •
edited
Loading

…mance

  • Increase default java_thread_count from 4 to 8 for faster parallel execution
  • Reduce default java_thread_timeout from 15 min (900000ms) to 30 sec (30000ms)
  • Update Java EnvUtil.java defaults to match TypeScript config.ts defaults
  • Both values remain configurable via CLI --sfge-thread-count and --sfge-thread-timeout flags or via engines.sfge section in code-analyzer.yaml

@nikhil-mittal-165
CHANGE - Tune SFGE default thread count and timeout for better perfor…
dd3dba5 
…mance

- Increase default java_thread_count from 4 to 8 for faster parallel execution
- Reduce default java_thread_timeout from 15 min (900000ms) to 3 min (180000ms)
- Update Java EnvUtil.java defaults to match TypeScript config.ts defaults
- Both values remain configurable via CLI --sfge-thread-count and --sfge-thread-timeout flags
  or via engines.sfge section in code-analyzer.yaml
@nikhil-mittal-165
test case fix
468a98f
@nikhil-mittal-165
Copy link
Contributor Author

nikhil-mittal-165 commented Mar 6, 2026

evidences

timeout =1min
threads = 8
execution time= 12 mins

image image

when
timeout =1min
threads = 2
execution time= 42 mins

image image

when
timeout = 3 min
threads = 8
execution time = 25 mins

image image

@nikhil-mittal-165
Copy link
Contributor Author

nikhil-mittal-165 commented Mar 9, 2026

timeout 30 seconds and thread count = 8
execution time is 7.5 minutes
violations : 378

image image

timeout 1 seconds and thread count = 16
execution time is 4 minutes
violations : 35

image image

namrata111f
namrata111f reviewed Mar 11, 2026
View reviewed changes
packages/code-analyzer-sfge-engine/sfge/src/main/java/com/salesforce/config/EnvUtil.java
@VisibleForTesting
static final long DEFAULT_RULE_THREAD_TIMEOUT =
TimeUnit.MILLISECONDS.convert(15, TimeUnit.MINUTES);
TimeUnit.MILLISECONDS.convert(30, TimeUnit.SECONDS);
Copy link
Contributor

@namrata111f namrata111f Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the PR description we say we are keeping this as 3 mins but are changing this to 30 seconds here?

Copy link
Contributor Author

@nikhil-mittal-165 nikhil-mittal-165 Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated description

@namrata111f
Copy link
Contributor

namrata111f commented Mar 11, 2026 •
edited
Loading

1- From what I understand this java_thread_count increase will cause more cpu consumption on the user side. Will this have any memory consumption impact on the user end? Also I am not sure if every user hardware will allow for 8 threads to be spawned, in that case to which number we will fallback to?
2- What is the use of changing java_thread_timeout? Is that to ensure no thread is stuck for more than that time? And if a thread is terminated in between how will it's work be continued later?

@nikhil-mittal-165
Copy link
Contributor Author

nikhil-mittal-165 commented Mar 11, 2026

  1. the memory consumption will be not very significant as for running sfge already requires significant heap memory
    and 1 thread stack will have 1 MB which should be managable
    JVM can spawn 8 threads in a single core machine as well with some context switching, so this behaviour should not break anything

  2. by chabging java_thread_timeout we interrupt threads which go on in exponential path expansion for a single entry point and free up the thread to be used by some other entry point. thread terminated will not affect other timeouts this is the current behaviour as well for 15 min timeout

@namrata111f
Copy link
Contributor

namrata111f commented Mar 11, 2026 •
edited
Loading

which go on in exponential path expansion for a single entry point and free up the thread to be used by some other entry point

So we are doing this to stop at around 93% of path expansion from single entry point as you had explained? Also I am assuming we would be just missing out on 6-7% of the violation in the remaining path and it's not like the final end vertex needs to be mandatory reached.

@nikhil-mittal-165
Copy link
Contributor Author

nikhil-mittal-165 commented Mar 11, 2026

yes 93% is the total overall violations caught, once the thread gets interrupted we discard that entry point altogether and do not check violations, this is the current timeout behaviour which remains unchanged

@namrata111f
Copy link
Contributor

namrata111f commented Mar 11, 2026

Ack, just one recommendation to test with some other large code bases as well to ensure this 93% theory in 30s of thread timeout hold good there as well.

@nikhil-mittal-165 nikhil-mittal-165 merged commit 2fadc1a into feature/static-rule-optimization Mar 12, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aruntyagiTutu aruntyagiTutu aruntyagiTutu approved these changes

@namrata111f namrata111f namrata111f approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@nikhil-mittal-165 @namrata111f @aruntyagiTutu